Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IMAGE_REF result from image building Tasks #1111

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

zregvart
Copy link
Member

@zregvart zregvart commented Jul 3, 2024

This helps in the reuse of the results when using matrix feature of Tekton. Given that the concatenation of two results from matrix-spawned Tasks is not supported, e.g.

$(tasks.build-container-multiarch.results.IMAGE_URL[*])@$(tasks.build-container-multiarch.results.IMAGE_DIGEST[*])

will not expand correctly.

This produces the image reference in full in the IMAGE_REF result, so the result from the matrix-spawned Tasks can be referenced using:

$(tasks.build-container-multiarch.results.IMAGE_REF[*])

Reference: https://issues.redhat.com/browse/EC-654

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

@arewm
Copy link
Member

arewm commented Jul 3, 2024

Is the intention of this to make it easier for users to onboard to a multi-arch flow or just to simplify the interface? If the former, then this process can also be simplified when we create a dedicated multi-arch Tekton Pipeline. The yaml files pushed out can look something similar to what I am using here for example: https://github.com/konflux-ci/buildah-container/pull/9/files.

@zregvart
Copy link
Member Author

zregvart commented Jul 4, 2024

@arewm the intention here is to enable the use of matrix feature which reduces the complexity of a multi-arch Pipeline definition. This was already presented on the May 30th demo call, you can lookup the recording.

@zregvart
Copy link
Member Author

Keeping this in draft until we get to STONEBLD-2326

@arewm
Copy link
Member

arewm commented Jul 16, 2024

I just ran into an issue related to this. For the OLM konflux sample, I cannot "prepare" all architectures to run and then only enable the ones that I need with the current form. As long as I explicitly include all of the previous tasks in the parameters, the index image generation will be skipped if any of the results are empty.

Changing to this pattern will enable a more user-friendly flow.

arewm added a commit to arewm/olm-operator-konflux-sample that referenced this pull request Jul 16, 2024
Since the `build-image-index` depends on the results from each of the
architecture-specific builds, the task (and all resulting dependant
tasks) will be skipped if any of these are absent.

Using the intended workflow of enabling users to specify the
architectures to run with PipelineRun params will require this model to
change. The PR konflux-ci/build-definitions#1111
is a potentially promising way to resolve this issue.

Signed-off-by: arewm <[email protected]>
arewm added a commit to arewm/olm-operator-konflux-sample that referenced this pull request Jul 16, 2024
Since the `build-image-index` depends on the results from each of the
architecture-specific builds, the task (and all resulting dependant
tasks) will be skipped if any of these are absent.

Using the intended workflow of enabling users to specify the
architectures to run with PipelineRun params will require this model to
change. The PR konflux-ci/build-definitions#1111
is a potentially promising way to resolve this issue.

Signed-off-by: arewm <[email protected]>
@zregvart zregvart marked this pull request as ready for review July 22, 2024 10:02
@zregvart
Copy link
Member Author

Rebased on buildah 0.2 changes, should be ready for another round of reviews now.

task/source-build/0.1/source-build.yaml Outdated Show resolved Hide resolved
task/tkn-bundle/0.1/README.md Outdated Show resolved Hide resolved
task/tkn-bundle/0.1/tkn-bundle.yaml Outdated Show resolved Hide resolved
@zregvart zregvart force-pushed the issue/EC-654-alt branch 4 times, most recently from 52d4145 to f5049e7 Compare July 22, 2024 15:17
This helps in the reuse of the results when using matrix feature of
Tekton. Given that the concatenation of two results from matrix-spawned
Tasks is not supported, e.g.

    $(tasks.build-container-multiarch.results.IMAGE_URL[*])@$(tasks.build-container-multiarch.results.IMAGE_DIGEST[*])

will not expand correctly.

This produces the image reference in full in the `IMAGE_REF` result, so
the result from the matrix-spawned Tasks can be referenced using:

    $(tasks.build-container-multiarch.results.IMAGE_REF[*])

Reference: https://issues.redhat.com/browse/EC-654
@arewm
Copy link
Member

arewm commented Jul 22, 2024

/retest

@arewm
Copy link
Member

arewm commented Jul 22, 2024

@zregvart, were you intending to change the generation of the image index in this PR as well or will that be up to users to change in their own pipelines?

@zregvart
Copy link
Member Author

@zregvart, were you intending to change the generation of the image index in this PR as well or will that be up to users to change in their own pipelines?

I think this is entirely up to the users and out of scope of this PR. If we had a multi-arch template pipeline then I would update that to use the new results. Follow EC-714 for more on that.

@zregvart zregvart added this pull request to the merge queue Jul 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 23, 2024
@arewm arewm added this pull request to the merge queue Jul 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 23, 2024
@zregvart zregvart added this pull request to the merge queue Jul 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 23, 2024
@arewm arewm added this pull request to the merge queue Jul 23, 2024
Merged via the queue into konflux-ci:main with commit 6dd763a Jul 23, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants